Skip to content

ci: add windows tests#385

Closed
medyagh wants to merge 5 commits intogoogle:masterfrom
medyagh:add_windows
Closed

ci: add windows tests#385
medyagh wants to merge 5 commits intogoogle:masterfrom
medyagh:add_windows

Conversation

@medyagh
Copy link
Member

@medyagh medyagh commented Jan 15, 2026

This PR added windows test and I noticed tests were failing so I fixed the tests too

minikube will be moving to use this library and we would like to ensure it has good testing coverage on windows. kubernetes/minikube#22456

The mustParseGolden helper reads the testdata/diffs golden file to verify test output. On Windows, where files are often checked out with CRLF line endings, the existing parser failed to correctly handle the carriage return (\r), leading to incorrect header keys (e.g., Key\r) and mismatched values.

This patch adds a normalization step to replace all \r\n sequences with \n immediately after reading the file. This ensures the parser works consistently regardless of the OS or Git line-ending configuration.

@medyagh
Copy link
Member Author

medyagh commented Jan 15, 2026

would you plz review neild

…is will ensure the diff command with process substitution runs correctly on Windows
…e gofmt failure on Windows. Please push the changes and check the CI again.
@neild
Copy link
Collaborator

neild commented Jan 16, 2026

The test failure is because git on Windows is mangling the line endings of the testdata files.

We should just use the same .gitattributes file the various Go repos use (go, x/net, etc.), which tells git to never change the line endings of any files. I just made #386 to do that.

Is there a good reason to add windows to the CI matrix? This module doesn't do anything platform specific; testing on more operating systems doesn't seem likely to catch any real bugs.

@medyagh
Copy link
Member Author

medyagh commented Jan 21, 2026

The test failure is because git on Windows is mangling the line endings of the testdata files.

We should just use the same .gitattributes file the various Go repos use (go, x/net, etc.), which tells git to never change the line endings of any files. I just made #386 to do that.

Is there a good reason to add windows to the CI matrix? This module doesn't do anything platform specific; testing on more operating systems doesn't seem likely to catch any real bugs.

we switched to use this library minikube we would like all our libs to have multi platfrom tests to increase reliability or reduce build surprises in the future

@neild
Copy link
Collaborator

neild commented Jan 21, 2026

we switched to use this library minikube we would like all our libs to have multi platfrom tests to increase reliability or reduce build surprises in the future

This package already runs tests on two platforms (covering two CPU architectures). It doesn't do anything platform-specific and we don't want to run it on every platform Go supports (of which there are many) so we need to pick a cutoff somewhere. Unless there's some reason to think testing on Windows adds real coverage, it just seems like extra resource consumption and CI delay for no benefit.

If you want to run this package's tests in your own CI on whatever platforms you like, you can of course do that.

@medyagh
Copy link
Member Author

medyagh commented Jan 22, 2026

ok

@medyagh medyagh closed this Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants